-
Notifications
You must be signed in to change notification settings - Fork 29
User Settings: return the list of installed editors #1217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User Settings: return the list of installed editors #1217
Conversation
…r-settings-return-the-list-of-installed-editors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works for my limited use case of having PhpStorm and VS Code, but I didn't have a chance to test for other editors or systems.
One thing I'm wondering about is the amount of editors. Do we want to add a support for a long list of editors, and then commit to maintain and test it for all for future releases?
Thanks for adding tests!
Yes, I ran into the same issue, even having installed some apps I can only test on Mac. I'm working on setting up Windows and Linux VMs on my proxmox box, and will try to test my assumptions of these install paths before merging.
Yes, this is an interesting question. From a maintenance burden standpoint, I think it'd be better to have only a handful of options and add a custom option where the user can browse for an arbitrary executable on their system. On #63 (comment) this was discussed earlier but you shared that there we have app-specific code which would limit us to do something like that. Can you elaborate on this a bit? Is this related to the opening logic we do currently in Lines 1102 to 1128 in 4aad06b
|
src/lib/is-installed.ts
Outdated
| 'vscode' | ||
| 'phpstorm' | ||
| 'cursor' | ||
| 'windsurf' | ||
| 'nova' | ||
| 'webstorm' | ||
| 'sublime' | ||
| 'atom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of notes regarding this list:
- atom was archived in 2022
- nova is mac-only
We can remove the Linux part at this point, or just leave what was supported there. We don't officially support Linux and we don't provide a Linux binary, yet, and I think we shouldn't dedicate too much time for Linux-specific features at this point.
Yes, the reason is that we couldn't find a unified way to open the project in the editor. I you look into |
…r-settings-return-the-list-of-installed-editors
This lets the tests running on Windows use UNIX style paths.
…ors' of github.com:Automattic/studio into stu-363-user-settings-return-the-list-of-installed-editors
src/lib/is-installed.ts
Outdated
const installationPaths: Record< string, PlatformPaths > = { | ||
darwin: { | ||
vscode: [ 'Visual Studio Code.app' ], | ||
phpstorm: [ 'PhpStorm.app' ], | ||
cursor: [ 'Cursor.app' ], | ||
windsurf: [ 'Windsurf.app' ], | ||
nova: [ 'Nova.app' ], | ||
webstorm: [ 'WebStorm.app' ], | ||
sublime: [ 'Sublime Text.app' ], | ||
atom: [ 'Atom.app' ], | ||
iterm: [ 'iTerm.app' ], | ||
terminal: [ 'Terminal.app' ], | ||
}, | ||
linux: { | ||
vscode: [ '/usr/bin/code' ], | ||
phpstorm: [ '/usr/bin/phpstorm' ], | ||
cursor: [ '/usr/bin/cursor' ], | ||
windsurf: [ '/usr/bin/windsurf' ], | ||
nova: [], | ||
webstorm: [ '/usr/bin/webstorm' ], | ||
sublime: [ '/usr/bin/sublime' ], | ||
atom: [ '/usr/bin/atom' ], | ||
iterm: [], | ||
terminal: [], | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katinthehatsite when rebasing this PR after #1215 was merged, I found it easier to merge the list of terminals and apps into one list, and do the same path manipulations for these as the IDEs.
I only had some issues with the typing, currently I use InstalledApps | InstalledTerminals
where we need to be able to handle both. Longer term, I think it'd make sense to maintain just one type of the possible apps/terminals we support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I am fine with this approach for now considering that the terminal apps are still applications and are located at the same path as other applications 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @gcsecsey, for working on this. It looks great to me!
I do have one concern similar to what Wojtek mentioned. Are we sure we want to support this many editors right from the start? It might make testing and maintenance more challenging.
Would it make sense to start with a smaller set of editors and gradually expand support, either later on or during the polishing phase?
src/lib/tests/is-installed.test.ts
Outdated
let isInstalled: ( key: string ) => boolean; | ||
let mockPaths: string[]; | ||
|
||
// Reset mocks before each test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clean up the comments as they seem to be pretty self explanatory unless it is something that is not evident
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it might be too much to start off with this many editors. As I mentioned above, I don't see too much benefits of supporting Nova and Atom at all, and I think we can start with a smaller subset and itroduce the rest later. As we need to support PHPStorm and VSCode, I think adjacent editors to these would be the easiest to start off with. For now, I'll update the list to be these editors, then we can iterate on these later.
|
We can add these back later as needed.
We can add these back later as needed.
Related issues
Fixes https://linear.app/a8c/issue/STU-363/user-settings-return-the-list-of-installed-editors
Proposed Changes
Note
I tried out the new PR Cursor template (pesfPa-1sj-p2) to produce a more detailed description.
This PR enhances the IDE detection functionality to support a wider range of editors across different platforms (macOS, Windows, Linux) with improved reliability and test coverage.
The existing IDE detection was limited in scope and didn't account for the various ways different platforms store and name their applications. Each platform (macOS, Windows, Linux) has its own conventions for:
.app
bundles on macOS vs executable names on Linux)/Applications
vsProgram Files
vs/usr/bin
)While I attempted to find a more generalized solution, the fundamental differences between platforms made it necessary to implement platform-specific path detection. While more verbose, this approach ensures reliable detection across all supported platforms.
Added support for detecting additional editors:
Enhanced JetBrains IDE detection:
Added comprehensive test coverage:
Testing Instructions
Unit tests
The changes include a new test suite that covers:
Run the new tests using
npm run test -- src/lib/tests/is-installed.test.ts
. All tests should pass.The new tests contain some path manipulation that were causing some issue when run on Windows. After fixing these, I ran the tests both on Mac and Windows to double-check:

On Mac:
On Windows:

Manual testing
STUDIO_PREFERRED_EDITOR=true npm start
Pre-merge Checklist